Skip to content

feat: Add finish_reason field to StreamingChunk#9536

Merged
sjrl merged 20 commits intomainfrom
streaming_chunk_finish
Jun 25, 2025
Merged

feat: Add finish_reason field to StreamingChunk#9536
sjrl merged 20 commits intomainfrom
streaming_chunk_finish

Conversation

@vblagoje
Copy link
Copy Markdown
Member

@vblagoje vblagoje commented Jun 19, 2025 •
edited
Loading

Why:

Introduces a dedicated finish_reason field to StreamingChunk class to improve type safety and provide better developer experience when handling streaming finish reasons. This enhancement adds structured access to finish reasons while maintaining full backward compatibility with the existing meta-based approach.

What:

  • New Field: Added finish_reason: Optional[FinishReason] field to StreamingChunk class with strict typing
  • Type Safety: Created FinishReason Literal type with standard values ("stop", "length", "tool_calls", "content_filter", "tool_call_results")
  • Dual Support: Updated generators to populate both new field and legacy meta field for maximum compatibility
  • Enhanced Utilities: Modified utility functions to use the new structured field for better type safety
  • Comprehensive Testing: Added thorough test coverage for both access patterns and all finish reason values

How can it be used:

Both approaches currently work, but we recommend using the dedicated StreamingChunk field:

# Recommended approach with better type safety and future compatibility
if chunk.finish_reason == "stop":
    add_spacing()

# Legacy approach (works now, but not guaranteed in future versions)
if chunk.meta.get("finish_reason") == "stop":
    add_spacing()

How did you test it:

  • Dual Compatibility: Tested both field and meta access patterns work correctly in current version
  • Type Safety: Verified strict typing works with all standard finish reason values
  • Generator Coverage: Updated OpenAI and HuggingFace generators with appropriate type handling
  • Utility Functions: Confirmed utility functions use new field for improved type safety

Notes for the reviewer:

Dual Support Strategy: This implementation provides current support for both access patterns:

  • Structured Access: chunk.finish_reason provides type safety, better IDE support, and future compatibility
  • Legacy Access: chunk.meta["finish_reason"] works for existing code but will likely be removed in future versions
  • No Breaking Changes: All existing code continues to work without modification in current version

@vblagoje vblagoje added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Jun 19, 2025
@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels Jun 19, 2025
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Jun 19, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 15871955598

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 59 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.04%) to 90.219%
Files with Coverage Reduction New Missed Lines %
components/rankers/sentence_transformers_similarity.py 1 98.75%
components/generators/hugging_face_api.py 2 96.43%
components/routers/init.py 2 46.15%
dataclasses/init.py 2 23.08%
dataclasses/streaming_chunk.py 4 91.53%
components/generators/chat/openai.py 6 96.39%
components/generators/chat/hugging_face_api.py 10 94.79%
components/tools/tool_invoker.py 14 83.94%
utils/base_serialization.py 18 89.09%
Totals Coverage Status
Change from base Build 15819151860: 0.04%
Covered Lines: 11677
Relevant Lines: 12943
💛 - Coveralls

@vblagoje vblagoje removed the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Jun 19, 2025
@vblagoje vblagoje marked this pull request as ready for review June 19, 2025 13:42
@vblagoje vblagoje requested review from a team as code owners June 19, 2025 13:42
@vblagoje vblagoje requested review from dfokina and sjrl and removed request for a team June 19, 2025 13:42
@vblagoje vblagoje added this to the 2.15.0 milestone Jun 19, 2025
@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented Jun 19, 2025

@sjrl please review and make sure that PR and migration plan make sense to you 🙏

@julian-risch julian-risch removed this from the 2.15.0 milestone Jun 20, 2025
@julian-risch
Copy link
Copy Markdown
Member

julian-risch commented Jun 20, 2025

The issue corresponding to this PR is already in the 2.15.0 milestone. That's the reason why I removed the PR from the milestone again. It's still planned to be part of the release.

@sjrl
Copy link
Copy Markdown
Contributor

sjrl commented Jun 23, 2025

Hey @vblagoje thanks for your work on this! I'll do a more in-depth review today but first I had a few comments and questions.

  • I don't think we need to have a migration. As in I think it's fine if we always leave the raw openai finish reason in the metadata. I don't think there is much value in removing this information from there and if users want to continue using that value then they can. So I'd ask to drop all the warnings and plan on leaving the original finish reason in the metadata.
  • Could you also update the other Generators and ChatGenerators in haystack main to set finish_reason? So this would be the HuggingFace type Generators and ChatGenerators.

sjrl
sjrl reviewed Jun 23, 2025
View reviewed changes
Comment thread haystack/components/generators/utils.py Outdated
sjrl
sjrl reviewed Jun 23, 2025
View reviewed changes
Comment thread haystack/dataclasses/__init__.py Outdated
sjrl
sjrl reviewed Jun 23, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
sjrl
sjrl reviewed Jun 23, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
@vblagoje vblagoje requested a review from sjrl June 23, 2025 13:37
sjrl
sjrl reviewed Jun 24, 2025
View reviewed changes
Comment thread haystack/components/generators/chat/hugging_face_api.py Outdated
sjrl
sjrl reviewed Jun 24, 2025
View reviewed changes
Comment thread haystack/components/generators/hugging_face_api.py Outdated
sjrl
sjrl reviewed Jun 24, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
sjrl
sjrl reviewed Jun 24, 2025
View reviewed changes
Comment thread test/dataclasses/test_streaming_chunk.py Outdated
@sjrl
Copy link
Copy Markdown
Contributor

sjrl commented Jun 24, 2025

@vblagoje could we also drop the migration and deprecation info in the PR description?

@sjrl sjrl enabled auto-merge (squash) June 25, 2025 09:01
@sjrl sjrl merged commit 91094e1 into main Jun 25, 2025
21 checks passed
@sjrl sjrl deleted the streaming_chunk_finish branch June 25, 2025 09:06
@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented Jun 26, 2025

Thank you @sjrl - much appreciate taking care of this one through the finish line!

@sjrl sjrl mentioned this pull request Aug 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjrl sjrl sjrl approved these changes

@dfokina dfokina Awaiting requested review from dfokina dfokina was automatically assigned from deepset-ai/documentation

Assignees

No one assigned

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add finish_reason field to StreamingChunk

4 participants

@vblagoje @coveralls @julian-risch @sjrl